Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat support existing ws endpoint #31

Merged
merged 6 commits into from
Oct 18, 2023
Merged

Feat support existing ws endpoint #31

merged 6 commits into from
Oct 18, 2023

Conversation

lowlighter
Copy link
Contributor

Closes #21

This makes it possible to use astral with services that offers remote browsers like https://www.browserless.io/

@lowlighter lowlighter marked this pull request as ready for review October 13, 2023 00:28
@@ -96,7 +104,8 @@ export class Browser {
url: "",
});
const browserWsUrl = new URL(this.#celestial.ws.url);
const wsUrl = `${browserWsUrl.origin}/devtools/page/${targetId}`;
const wsUrl =
`${browserWsUrl.origin}/devtools/page/${targetId}${browserWsUrl.search}`;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The browserWsUrl.search is required because some service may use this for authentication
For example browserless.io:

  const browser = await puppeteer.connect({
    browserWSEndpoint: `wss://chrome.browserless.io?token=******`,
  });

@lino-levan
Copy link
Owner

Have you tested this out? I'm slightly concerned because we do make fetch requests to the URL in some places throughout the code (here is an example). I have no idea how a service like this could work across network requests.

@lowlighter
Copy link
Contributor Author

Yeah I have tested with the following code snippet:

import { launch } from "https://esm.sh/gh/lowlighter/astral@feat-support-existing-ws-endpoint/mod.ts";
const browser = await launch({
  browserWSEndpoint:
    "wss://chrome.browserless.io?token=******",
});
const page = await browser.newPage("http://example.com");
console.log(await page.evaluate(() => document.title));
await browser.close();

And it does work (at least with browserless.io, and an existing local browser as highlighted by the tests):

@lowlighter ➜ /workspaces/astral (feat-support-existing-ws-endpoint) $ ~/.deno/bin/deno run -A test.ts
Example Domain

However as you mentioned calling page.close() directly will fail as it cannot reach /json/close, but since it cleans the celestial websocket anyways, the tests weren't failing because there were no leaking ops as resources were still properly sanitized.

I'll make a few changes to address this 👍

Copy link
Owner

@lino-levan lino-levan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the test!

src/browser.ts Outdated Show resolved Hide resolved
src/browser.ts Outdated Show resolved Hide resolved
@lino-levan
Copy link
Owner

Actually, if you could write a section in the docs about "connecting to remote browsers", that would be amazing!

Copy link
Owner

@lino-levan lino-levan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@lino-levan lino-levan merged commit 484c45a into lino-levan:main Oct 18, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

suggestion: ability to connect to arbitrary WS address
2 participants